Skip to content

Conversation

@Abogical
Copy link
Member

@Abogical Abogical commented Oct 10, 2025

Reason for this change

  • Previously, the script was executing the script under the cwd of the script package, when it should be at the head branch folder.
  • PATH was not passed on to integration test spawn command, leading to a ENOENT error as the integration test command could not find yarn.
  • Workflow trigger on label is misconfigured.

Description of changes

  • We'll use node explicitly to run the script without changing the cwd.
  • PATH env variable is now passed to integration test command.
  • Workflow trigger on label is fixed.

Describe any new or updated permissions being added

No new permissions were added.

Description of how you validated changes

Tested on my fork here: https://github.com/Abogical/aws-cdk/actions/runs/18465794651/job/52607352315?pr=15

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@github-actions github-actions bot added the admired-contributor [Pilot] contributed between 13-24 PRs to the CDK label Oct 10, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team October 10, 2025 15:17
@github-actions github-actions bot added the p2 label Oct 10, 2025
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Oct 10, 2025
@Abogical Abogical force-pushed the integ-deployment-test-invoke-fix branch 7 times, most recently from bdc479e to 5626551 Compare October 13, 2025 12:26
@Abogical Abogical changed the title ci: fix integration-test-deployment script invocation ci: fix integration-test-deployment scrip Oct 13, 2025
@Abogical Abogical changed the title ci: fix integration-test-deployment scrip ci: fix integration-test-deployment script Oct 13, 2025
@Abogical Abogical marked this pull request as ready for review October 13, 2025 13:03
@aws-cdk-automation aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Oct 13, 2025
fetch-depth: 0
path: head

- name: Configure AWS credentials
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have the OIDC? Unless there's some concern on it that i'm not aware of

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OIDC added back in. I'm currently testing in my fork if it still works.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

workflow_dispatch: {}
merge_group: {}
pull_request_target:
types:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the default behaviour if we didn't mention any?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It won't listen to any labelling events by default. So if we add the pr/needs-integration-test-deployment label, the workflow will still be skipped.

"bump": "./bump.sh",
"build-all": "tsc -b"
"build-all": "tsc -b",
"atmosphere-integ-test": "lerna run build --scope @aws-cdk/integration-test-deployment && node tools/@aws-cdk/integration-test-deployment/bin/index.js"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it building? I believe the normal pattern here is another separate command building, then this one only running

Copy link
Member Author

@Abogical Abogical Oct 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • pkglint above in package.json follows the same pattern.
  • It'll be helpful as a developer to have one command to rebuild and test the script to allow for faster development.

@Abogical Abogical requested a review from gasolima October 13, 2025 15:50
@mergify
Copy link
Contributor

mergify bot commented Oct 15, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot added the queued label Oct 15, 2025
@mergify
Copy link
Contributor

mergify bot commented Oct 15, 2025

This pull request has been removed from the queue for the following reason: checks failed.

The merge conditions cannot be satisfied due to failing checks:

You may have to fix your CI before adding the pull request to the queue again.
If you update this pull request, to fix the CI, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@mergify mergify bot removed the queued label Oct 15, 2025
@Abogical
Copy link
Member Author

@Mergifyio rebase

- Previously, the script was executing the script under the cwd of the script package, when it should be at the head branch folder. We'll use node explicitly to run the script without changing the cwd.

- Fixes the label and event conditions that would trigger the workflow.

- Use STS to assume Atmosphere role as OIDC is not needed when we use the codebuild runner.
PATH environment variable is used to find yarn/node binaries. Without it, a ENOENT error is emitted when running yarn.
Running via node directly instead of lerna will keep the cwd at the root as expected.
@mergify
Copy link
Contributor

mergify bot commented Oct 15, 2025

rebase

✅ Branch has been successfully rebased

@Abogical Abogical force-pushed the integ-deployment-test-invoke-fix branch from b37d62c to 31f9616 Compare October 15, 2025 15:11
@mergify
Copy link
Contributor

mergify bot commented Oct 15, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot added the queued label Oct 15, 2025
@aws-cdk-automation aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Oct 15, 2025
@mergify
Copy link
Contributor

mergify bot commented Oct 15, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify
Copy link
Contributor

mergify bot commented Oct 15, 2025

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit c53de0e into main Oct 15, 2025
18 of 19 checks passed
@mergify mergify bot deleted the integ-deployment-test-invoke-fix branch October 15, 2025 16:38
@mergify mergify bot removed the queued label Oct 15, 2025
@github-actions
Copy link
Contributor

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

admired-contributor [Pilot] contributed between 13-24 PRs to the CDK contribution/core This is a PR that came from AWS. p2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants